Repository navigation
feat(mysql): table DDL and generated migrations, with a migration fuzzer (#53) - #136
Conversation
MySQL.table(name, { columns, primaryKey, indexes, tenantColumn, casing })
returns a MysqlSchemaTable, as SQLite.table does, with column, index and
uniqueIndex. /schema gains mysqlEntitiesOf, renderMysqlSchema,
diffMysqlSchemas, renderMysqlOp and the MySQL entities and snapshot.
Tables are CREATE TABLE IF NOT EXISTS ... ENGINE=InnoDB DEFAULT
CHARSET=utf8mb4 with the primary key and UNIQUE constraints inside, and
separate CREATE INDEX statements. A literal default on TEXT, BLOB or JSON
is written DEFAULT ('...'). Definitions record problems for TEXT, BLOB and
JSON in keys, nullable key columns, keys past 3072 bytes, names past 64
characters and colliding index names.
The diff alters in place: ADD / DROP / MODIFY COLUMN (filling NULLs with
the default before NOT NULL), DROP / ADD PRIMARY KEY around the column
changes, index and UNIQUE drops before them and adds after, RENAME TABLE,
and CHANGE COLUMN for a rename: MariaDB keeps a JSON column's
json_valid CHECK under the old name through RENAME COLUMN.
The /migrate runner, verify and verifySnapshot refuse a MySQL folder or
snapshot, as they refuse SQLite's: no ledger or catalog reader yet.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
generate diffs MySQL.table definitions against the folder and writes <timestamp>_<name>/migration.sql plus a mysql snapshot.json, one statement per --> statement-breakpoint, as for SQLite. verify --schema refuses a MySQL config before building its driver, and --from-drizzle refuses dialect "mysql". A drizzle-kit snapshot naming mysql is read as MySQL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Random before/after schemas (types, nullability, defaults, keys, UNIQUE columns, indexes, table and column renames) are created in throwaway databases on MySQL 8.4 and MariaDB 11.4. The migration runs on seeded rows; information_schema must match a fresh create of the new schema, and the kept columns their seeded values. Pins the MariaDB RENAME COLUMN input that found the json_valid CHECK name drift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This comment has been minimized.
This comment has been minimized.
…ClickHouse --from-drizzle message Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Maple review🟡 Confidence 6/10 · needs attention. Held at 6 because a correctness warning is open. MySQL gets table DDL, an in-place schema diff and
Before merge
Findings🟠 Warning · F3 ·
|
| Issue | Service | Occurrences | File |
|---|---|---|---|
| ToolCallFailed | maple-security-audit | 838 | src/schema.ts |
| @maple/http/errors/WarehouseAuthError | maple-web | 20 | src/schema.ts |
| ContextBudgetError | maple-security-audit | 8 | src/schema.ts |
| ModelProtocolError (2 issues) | maple-chat | 11 | src/schema.ts |
| AgentPolicyError | maple-chat | 7 | src/schema.ts |
| @maple/security-audit/AgentError | maple-security-audit | 4 | src/schema.ts |
| Error | maple-web | 4 | src/schema.ts |
| ContextBudgetError | maple-chat | 3 | src/schema.ts |
After this merges, Maple checks whether they stop.
d1d37ad · What was checked, coverage and telemetry details are on the Maple / review check. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
![maple-review-bot[bot]](https://avatars.githubusercontent.com/in/4108169?s=60&v=4){kind=small}
| const modify = (to: MysqlColumnEntity) => `ALTER TABLE ${mysqlIdent(column.table)} MODIFY COLUMN ${renderMysqlColumnDefinition(to)}` | ||
| // Strict mode refuses NOT NULL over existing NULLs; they take the default, as a new row | ||
| // would. The column takes its new type first, so the default is a value of it. | ||
| return op.fill && column.default !== null |
There was a problem hiding this comment.
Warning
fill with no default writes MODIFY … NOT NULL that fails on existing NULLs
F3 · Warning · correctness
Making an existing nullable column NOT NULL with no default leaves op.fill true while column.default is null, so only MODIFY COLUMN … NOT NULL is written. Strict mode refuses it over any NULL row (the comment above says so), and MySQL DDL is not transactional, so the statements before it in migration.sql stay applied.
In `diffMysqlSchemas`, when `fill` is needed and `column.default === null`, push an `unsupported` entry instead of the op, so `generate` refuses the change rather than writing a statement that aborts on rows.
🤖 Prompt to fix with an AI agent
In `src/schema/mysql-ops.ts:75-81`: `fill` with no default writes `MODIFY … NOT NULL` that fails on existing NULLs.
Making an existing nullable column NOT NULL with no default leaves `op.fill` true while `column.default` is null, so only `MODIFY COLUMN … NOT NULL` is written. Strict mode refuses it over any NULL row (the comment above says so), and MySQL DDL is not transactional, so the statements before it in `migration.sql` stay applied.
Suggested fix: In `diffMysqlSchemas`, when `fill` is needed and `column.default === null`, push an `unsupported` entry instead of the op, so `generate` refuses the change rather than writing a statement that aborts on rows.
Verify the problem exists at that location before changing it, and keep the fix to those lines.
There was a problem hiding this comment.
Won't fix. Making a column NOT NULL with no default is a valid migration whenever the column holds no NULLs: an empty table, or one that was backfilled first. Refusing it would block that common case, and nothing in the schema says whether NULLs exist. SQLite's rebuild behaves the same way (the copy fails on NULLs), and drizzle-kit writes the same statement. docs/migrations.md warns about this case in the MySQL section ("a NOT NULL with no default over NULLs" fails, and DDL is not transactional).
|
Merged with 1 step from "Before merge" still open. If it is done, tick it on the review comment; if not, now is the time.
|
Maple: after this shippedDeployed to New errors since the deploy:
Errors in the changed files:
Maple looks once, an hour after the first deploy that carries the merge. |
Stacked on #135, so review and merge that one first. This gives MySQL and MariaDB table DDL and generated migrations, the same way SQLite has them, plus a live migration fuzzer.
What changed
MySQL.tablecan declare DDL.MySQL.table(name, { columns, primaryKey, indexes, tenantColumn, casing })now carries its DDL, using the same overloads asSQLite.table. The value is still a queryTable, withddlandproblemsbeside it.MySQL.column,MySQL.indexandMySQL.uniqueIndexgo with it, and{ external: true, columns }declares a table another tool owns.DEFAULT ('…')./schemaadds the MySQL migration pieces:mysqlEntitiesOf,renderMysqlSchema,diffMysqlSchemas,MysqlMigrationOp,renderMysqlOp,renderMysqlMigrationSqlandMysqlSnapshot.ADD,DROP,MODIFYandCHANGE COLUMN, primary key and index changes, andRENAME TABLE.confirm_data_loss.effect-orm generatewithdialect: "mysql"writes the drizzle-kit 1.0 folder layout, one statement per breakpoint. As with SQLite, effect-orm does not apply these migrations yet:/migrate,verify --schemaand--from-drizzlerefuse MySQL clearly.tests/fuzz/migrations.mysql.fuzz.test.tsgenerates random before/after schemas: nine column types, nullability, defaults, keys, unique and indexed columns, and table and column renames, including case-only ones. For each it:beforeinformation_schemato match a fresh create ofafter, with the seeded values of kept columns intactsrc/andtests/fuzz/, which includes the new migration fuzzer, are picked up by the existing filters.What the fuzzer found
CHECK (json_valid(...))named after the column, andRENAME COLUMNleaves the old name in place, so a migrated database differed from a fresh create. Renames are now written asCHANGE COLUMN. Pinned inregressions.ts.How it was verified
FUZZ_SCALE=20 FUZZ_VERBOSE=1checked 600 of 600 inputs on MySQL 8.4 and 600 of 600 on MariaDB 11.4, with nothing skipped. In a 300-input sample every input produced a migration, 132 ran on seeded rows, and every op kind appeared.tests/ddl.mysql.test.ts) apply the rendered DDL on both servers and read every default back through the builder. A livegeneratetest checks that migrations applied in order match a fresh create.src/mysql/ddl.test.ts(24 tests),src/kit/mysql-generate.test.ts, and the extendedsrc/mysql/mysql.test-d.ts.bun run build,bun run typecheckandbun run testpass (1,576 passed; the live suites skip without servers), along with the export catalog (216/mysqlexports), doc citations and 40 typechecked doc examples.Not in this PR
--from-drizzlefor MySQLverify, drift)AUTO_INCREMENT, foreign keys, CHECK constraints, prefix and expression indexes, and column collationsAFTERplacement for added columnsA new NOT NULL column without a default fills existing rows with the type's implicit default. This is documented, not refused.
Part of #53.
🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.